test(reservations): M2 test-coverage backfill (7 of 8 items) - #4280

Blocker gap found while preparing the M3 multi-signer integration
test: pkg/tbtc/coordination.go's getActionsChecklist decides which
WalletActionTypes a coordination round even considers, and it never
emitted ActionReservationAnchor or ActionReservationReanchor.

pkg/tbtcpg.ProposalGenerator.Generate only runs a task whose
ActionType() appears in the checklist it's handed (tbtcpg.go:124-135)
- it never iterates pg.tasks directly. NewReservationAcceptanceTask
and NewReservationReanchorTask are registered when
config.Reservations.Enabled=true (tbtcpg.go:88-92), but with no
checklist entry, Generate's per-window loop never selected them.
Both tasks were structurally unreachable in production regardless of
PR #4276/#4277's fixes.

Every existing unit test for these tasks (reservation_acceptance_test.go,
reservation_reanchor_test.go) calls task.Run(request) directly,
bypassing getActionsChecklist/Generate entirely - which is why this
was never caught by any prior PR's test suite.

Fix: ActionReservationAnchor and ActionReservationReanchor are now
appended unconditionally, checked on every coordination window like
ActionRedemption (both are custody-critical - an unaccepted
reservation or a stale re-anchor risks stranding, not just reduced
throughput - unlike the frequency-gated sweep/moving-funds actions).
A node with reservations disabled safely no-ops on these: Generate
already treats a checklist action with no matching registered task
as 'unsupported' and skips it without error (tbtcpg.go:131-135), the
same mechanism that already gates every other optional per-node task.

Testing:
- Updated TestCoordinationExecutor_GetActionsChecklist and its
  _PostActivation sibling: every non-nil expected checklist now
  includes both new actions right after ActionRedemption, matching
  the real append order. Extended assertChecklistOrdering's priority
  map accordingly (Redemption=0, ReservationAnchor=1,
  ReservationReanchor=2, then the existing sweep/moving-funds/
  heartbeat priorities shifted).
- Added TestCoordinationExecutor_GetActionsChecklist_ReservationActionsAlwaysPresent,
  a dedicated regression guard asserting both actions are present
  across pre/post-activation and 4th/non-4th windows, decoupled from
  the large table-driven test - would fail on its own if this wiring
  regresses.
- go test ./pkg/tbtc/... ./pkg/tbtcpg/...: 476/476 pass.
- go build ./... && go test ./...: full repo, 49 packages, zero FAIL.
- gofmt/vet clean on both changed files.

…oordination

Implementation-plan.md Milestone 3, 'multi-signer simulated
integration test' item (per user decision: build the test, leave the
testnet-drill item as an agent-not-actionable tracked item since it
needs live infra and calendar time, not code).

Scales TestCoordinationExecutor_Coordinate's existing 3-operator
harness - deterministic keypairs, real per-operator localChain fakes,
a real shared netlocal.BroadcastChannel, one goroutine per operator
running coordinationExecutor.coordinate concurrently - to
ReservationAnchorProposal and ReservationReanchorProposal. This
exercises the real leader/follower coordination round-trip
(checklist generation -> leader election -> broadcast -> follower
validation -> convergence) that no mocked pkg/tbtcpg unit test can
cover, since those call task.Run(request) directly and never go
through coordinationExecutor.coordinate. It also exercises PR
#4277's protobuf marshaling of both proposal types over a real wire
round-trip, since every follower unmarshals the leader's broadcast
coordinationMessage.

Depends on PR #4278 (this branch's parent): before that fix,
ActionReservationAnchor/ActionReservationReanchor never appeared in
getActionsChecklist's output, so every operator's checklist search in
these tests would fall through to NoopProposal and fail - confirmed
by temporarily reverting the checklist fix and re-running (both new
tests failed with the expected NoopProposal mismatch), then restoring
it.

Found and fixed one bug in this test's own harness during
verification: both new tests initially shared one netlocal broadcast
channel name. getBroadcastChannel's registry is keyed by name and
never releases old channels, so under -race (which changed
goroutine/channel-delivery timing enough to surface it in ~every
run), the reanchor test's follower sometimes received a stale
broadcast left over from the anchor test's leader. Fixed by giving
each test its own channel name; re-verified stable across 10
repeated -race runs plus the full non-race and race suites.

Testing:
- go test ./pkg/tbtc/...: 365/365 pass.
- go test -race ./pkg/tbtc/...: clean, no data races, including
  -count=10 on just the two new tests.
- go build ./... && go test ./...: full repo, 49 packages, zero FAIL.
- gofmt -l / go vet: clean.

Implementation-plan.md Milestone 2, all items except the one item
(ValidateReservationAnchorProposal/ValidateReservationReanchorProposal
tests) that needs simulated-backend test infrastructure this
repository doesn't have - see docs/spec/reservations/m1-keep-core-
readiness/01-gap-analysis.md's new Minor row for that finding, and
the item-8 dedup row for the golden-value equivalence decision.

pkg/tbtc/reservation_test.go:
- TestAssembleReservationAnchorTransaction: happy-path output shape
  (1-in-1-out, deposit value minus fee, P2WPKH to the target wallet).
  Also doubles as the golden reference for pkg/tbtcpg's dedup test.
- TestAssembleReservationReanchorTransaction: same shape assertion
  for the re-anchor sibling.

pkg/chain/ethereum/tbtc_test.go:
- TestConvertReservationParametersFromAbiType: full 10-tuple field
  mapping, every field set to a distinct non-zero value so a swapped
  or dropped field can't hide behind a shared zero default.
- TestConvertReservationFromAbiType_DropsCumulativeReanchorFee: pins
  the intentional CumulativeReanchorFee omission and verifies every
  other field maps correctly around it.

pkg/tbtcpg/reservation_acceptance_test.go:
- TestReservationAcceptanceTask_BoundaryChecks: 6-case table
  (at-limit accepts / one-over rejects) for MaxReservationsPerWallet,
  ReservationMinAmount, and ReservationMaxTotalAmount -
  BoundedLookback only ever used these as fixture data, never at the
  actual boundary.
- TestReservationAcceptanceTask_ReservationParametersFetchedLive:
  runs the same task twice against the same deposit, mutating
  ReservationMinAmount between calls - proves ReservationParameters()
  is fetched live per call, not cached on the task.

pkg/tbtcpg/reservation_anchor_dedup_test.go (new file, package
tbtcpg - internal, not tbtcpg_test - to reach the unexported
function):
- TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput:
  buildReservationAnchorTransaction (pkg/tbtcpg) and
  assembleReservationAnchorTransaction (pkg/tbtc) are independently
  maintained copies of the same logic, both unexported in different
  packages - Go's visibility rules make a single test calling both
  impossible without a production-code change. This test and
  pkg/tbtc's TestAssembleReservationAnchorTransaction instead pin the
  identical golden input/output values (deposit 100000, fee 1500,
  output 98500) in each package, catching either copy drifting from
  the other without eliminating the underlying duplication (real fix
  deferred, per decision this session).

Testing:
- go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/chain/ethereum/...:
  519/519 pass.
- go build ./... && go test ./...: full repo, 49 packages, zero FAIL.
- gofmt -l / go vet: clean on all 4 changed/new files.

…-coordination-checklist

…ion-multisigner-integration-test

…servation-test-coverage-backfill

…tobuf-marshaling

Reconciles this branch's protobuf-marshaling migration for reservation
proposals with the review-fix commit that landed independently on the
base branch, which performed the same migration plus additional
correctness fixes and a scope narrowing.

- message.proto/message.pb.go: drop ReservedRedemptionProposal and
  ReservationDissolutionProposal (base commit 757c6d8: 'scaffolding
  that ships with no producer, dispatch case, or wired validator in
  this milestone' - M1 is creation/custody/re-anchor only per the
  variant-B scope decision); regenerated message.pb.go from the
  resolved .proto with the pinned protoc/protoc-gen-go toolchain.
- reservation.go/marshaling.go: adopt the base's Marshal/Unmarshal for
  the two surviving types (anchor, re-anchor), which restores the
  all-zero TargetWalletPublicKeyHash guard this branch's independent
  migration had not carried; drop this branch's now-superseded
  duplicate implementation and the two removed types' scaffolding;
  added back the doc comments the base's version had dropped, matching
  every other proposal type's convention in the file.
- reservation_test.go: adopt the base's renamed, restructured
  TestReservationProposals_UnmarshalRejectsMissingIntegers (matching
  its 2-arg marshalPb(t, msg) helper and null-payload style); restored
  two byte-length validation test cases
  ('invalid deposit funding tx hash length',
  'invalid target wallet public key hash length') the base's version
  had dropped despite the corresponding Unmarshal checks still being
  live - both now pass against the merged validation order.
- marshaling_test.go: dropped the two removed types' table entries and
  dedicated fuzz round-trip tests; fixed a duplicate proto import left
  by the auto-merge.

Full repo: zero FAIL.

…-coordination-checklist

Reconciles this branch's original coordination-checklist wiring with
the review-fix commit that independently re-fixed the same wiring on
the base branch (757c6d8: 'gate reservation checklist entries solely
on the activation block, not the local per-operator config flag').

- coordination.go: the auto-merge silently duplicated the reservation
  checklist append (this branch's original unconditional append,
  right after Redemption, plus the base's corrected
  activation-block-and-frequency-gated append later in the function) -
  ActionReservationAnchor/ActionReservationReanchor would have been
  listed twice whenever both conditions held, and listed even before
  activation the rest of the time. Removed this branch's unconditional
  append; kept only the base's gated one.
- coordination_test.go: adopted the base's
  TestCoordinationExecutor_GetActionsChecklist_Reservations (activation
  + frequency gating) and its updated priority ordering
  (Redemption < DepositSweep < MovedFundsSweep < MovingFunds <
  ReservationAnchor < ReservationReanchor < Heartbeat, reservations now
  gated like the sweep actions rather than unconditional). Rewrote this
  branch's two pre-existing checklist tests
  (TestCoordinationExecutor_GetActionsChecklist,
  _PostActivation) to match: expected values recomputed by running the
  corrected production getActionsChecklist against each test's exact
  block/window inputs, not hand-derived - 24 subtests total, all
  independently verified against the same fixed implementation the
  regression guard exists to catch drift in.

Full repo: zero FAIL.

…ion-multisigner-integration-test

Auto-merged cleanly on coordination_test.go (this branch's own
additions - the multi-signer integration tests and their fixtures -
land purely after the base's content with zero line overlap), but the
merge exposed a real breakage in this branch's own new tests: both
TestCoordinationExecutor_Coordinate_ReservationAnchor and
_ReservationReanchor used coordinationBlock = 900, which predates
ReservationsActivationBlock (24559289) fixed on the base branch
(757c6d8). With the base's activation-block gate now in effect,
ActionReservationAnchor/ActionReservationReanchor never appear in the
checklist at block 900, so both tests' mock proposal generators fell
through to NoopProposal and failed.

Bumped both tests' coordinationBlock to 24559289+3511=24562800 (a
post-activation, 4th-coordination-window block, verified against the
same production getActionsChecklist this session already confirmed
correct on the base branch). Leader election is unaffected by this
change and stays at operator2: the coordination seed depends only on
the wallet public key hash and the injected safe-block hash - both
fixed regardless of which raw block number the test passes in - not on
the coordination block number itself. Corrected
newReservationCoordinationWallet's doc comment, which had claimed
leader election was tied to block 900 specifically.

Full repo: zero FAIL.

…servation-test-coverage-backfill

Reconciles this branch's M2 test-coverage backfill work with several
independent changes on the base: dead-code removal, the conversion
function rename, and a new net-of-fee minimum-amount gate.

- Deleted pkg/tbtcpg/reservation_anchor_dedup_test.go entirely: its
  sole test pinned buildReservationAnchorTransaction (tbtcpg package)
  against a golden value shared with pkg/tbtc's sibling function, to
  catch drift between the two independently-maintained duplicate
  implementations. c48d17b deleted buildReservationAnchorTransaction
  from production code (reservation_acceptance.go now derives the
  reservation key from the anchor UTXO instead of reassembling the
  transaction) - the duplication this test existed to guard against no
  longer exists, so the guard is moot, not just broken.
- pkg/tbtc/reservation_test.go: fixed two tests
  (TestAssembleReservationAnchorTransaction,
  TestAssembleReservationReanchorTransaction) left calling the
  pre-rename lowercase assembleReservation{Anchor,Reanchor}Transaction;
  both functions were exported and gained a required
  *ReservationAction parameter (fee-ceiling guard) upstream. Updated
  call sites and doc comments; both tests' own value remains real
  (happy-path output-shape coverage the nearby InputValidation test
  doesn't provide).
- pkg/chain/ethereum/tbtc_test.go: this branch and the base
  independently added TestConvertReservationParametersFromAbiType with
  different fixture values (both equally rigorous); kept this branch's
  version (doc-linked to the gap-analysis finding it backfills) and
  the base's two genuinely new, non-overlapping tests
  (TestConvertReservationFromAbiType,
  TestConvertReservationActionFromAbiType).
- pkg/tbtcpg/reservation_acceptance_test.go: same collision pattern -
  this branch's TestReservationAcceptanceTask_ReservationParametersFetchedLive
  and the base's TestReservationAcceptanceTask_GetWalletError are
  unrelated tests that landed at the same insertion point; kept both.
  Fixed three latent fixture gaps this merge surfaced in this branch's
  own ReservationParametersFetchedLive/BoundaryChecks tests (pre-dating
  this merge, never run against current production code until now):
  missing EndBlock on the DepositRevealedEventFilter, missing Vault on
  the DepositRevealedEvent (both added by c48d17b's EndBlock/vault-
  match hardening), and a missing fee-oracle rate
  (SetEstimateSatPerVByteFee) now required by dynamic fee estimation.
  Also corrected the 'exactly at minimum' boundary case's deposit
  amount: proposeReservationAcceptance now enforces the minimum against
  the *net-of-fee* anchor value, not the gross deposit amount, so the
  boundary must be reservationMinAmount + the fixture's deterministic
  710-sat fee, not reservationMinAmount itself.

Full repo: zero FAIL.

…ration-test' into m1/reservation-test-coverage-backfill

# Conflicts:
#	pkg/tbtc/coordination.go
#	pkg/tbtc/coordination_test.go
#	pkg/tbtc/marshaling_test.go
#	pkg/tbtc/reservation_test.go
#	pkg/tbtcpg/reservation_acceptance_test.go

Fixes 20 P1/P2/P3 findings from a multi-agent review of this PR's own
test coverage:

pkg/tbtcpg/reservation_acceptance_test.go:
- Narrow the PastDepositRevealedEvents override to swallow only its
  claimed sentinel; add error-injection coverage.
- Assert AnchorTransactionAssembly's returned proposal fields against
  the task-derived candidate, not just a hand-built fixture object;
  make the fixture's ValidateReservationAnchorProposal genuinely check
  the funding outpoint instead of always returning nil.
- Delete the fixture's shadow ReservationParameters override/field;
  route through the base LocalChain's already-correct value-copy
  setter/getter instead.
- Fix a test comment overclaiming exclusion is "strictly attributable"
  to one filter when multiple fixture gaps independently cause it.
- Make the freshness-control test's RequestReservationAcceptance
  override actually record an event, so the dedup guard it's meant to
  exercise engages for real.
- Pin RequestNonce==1 for the documented GetReservation fail-open path.
- Make BoundaryChecks' three cap fields pointer-typed so a row can
  express production's real "0 = unlimited" semantic; add rows proving
  it.
- Delete the dead reservedDeposits field/IsReservedDeposit override
  (zero production readers).
- Add TestReservationAcceptanceTask_ValidateProposalError covering the
  previously-unexercised validateErr wrapping path.
- Fix two misleading comments (wrong function attribution for the
  ReservationMinAmount gate; wrong description of the GetWallet error
  contract).
- Add testReservationVaultAddress const, replacing 30+ inline literal
  duplicates.
- Extract newBoundaryTestChain helper, replacing a ~13-line setup block
  duplicated across 16 call sites.

pkg/tbtcpg/chain_test.go:
- Fix binary.BigEndian.PutUint64 writing EndBlock bytes into the
  startBlock buffer instead of endBlock across 5 call sites, so
  EndBlock actually contributes to the event-filter cache key.

pkg/chain/ethereum/tbtc_test.go:
- Replace a stale tbtc.go line-range doc citation with a symbol
  reference.
- Fold TestConvertReservationFromAbiType_DropsCumulativeReanchorFee
  into TestConvertReservationFromAbiType as a subtest, matching this
  file's one-test-per-converter convention.

pkg/chain/ethereum/tbtc.go:
- Add an in-tree TODO marking ValidateReservationAnchorProposal's
  deferred test coverage, since the docs describing that deferral
  don't exist in this checkout.

The 'records acceptance request, skipping duplicate on subsequent run'
test relied on hasPendingAction's fail-closed default on a missing
GetReservationAction record, not on the intended pending-state
detection path - its comment still described the removed
PastReservationAcceptanceRequestedEvents mechanism. Explicitly set
the action record to Pending after the first run so the second run's
dedup assertion genuinely exercises hasPendingAction's happy path.